Skip to content

[sw,cryptolib] Change expected values for tests with result 0 #28602

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Oct 31, 2025
Merged

[sw,cryptolib] Change expected values for tests with result 0 #28602

merged 1 commit into from
Oct 31, 2025
+202 −440

Conversation

@h-filali
Copy link
Contributor

@h-filali h-filali commented Oct 30, 2025 •
edited
Loading

This commit changes the expected values for the tests that have an expected result of 0. This makes sure we don't run into issues where the tests are not executing properly and we still get a correct result due to the expected value of 0.

Furthermore, this commit moves the tests to the hjson framework.

The input and output values don't change from before this change. This commit merely skips the unnecessary calculations.

Resolves #23343

@h-filali h-filali added the CherryPick:earlgrey_1.0.0 This PR should be cherry-picked to earlgrey_1.0.0 label Oct 30, 2025
@h-filali h-filali force-pushed the cryptolib-change-test-vec branch 2 times, most recently from a2f1870 to c8eb5aa Compare October 30, 2025 17:17
andrea-caforio
andrea-caforio approved these changes Oct 31, 2025
View reviewed changes
Copy link
Contributor

@andrea-caforio andrea-caforio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @h-filali. No functional change just test logic.

johannheyszl
johannheyszl approved these changes Oct 31, 2025
View reviewed changes
Copy link
Contributor

@johannheyszl johannheyszl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @h-filali

@h-filali h-filali force-pushed the cryptolib-change-test-vec branch from c8eb5aa to c762629 Compare October 31, 2025 08:43
@h-filali
[sw,cryptolib] Change expected values for tests with result 0
59173d6 
This commit changes the expected values for the tests that have an
expected result of 0. This makes sure we don't run into issues
where the tests are not executing properly and we still get a
correct result due to the expected value of 0.

Furthermore, this commit moves the tests to the hjson framework.

The input and output values don't change from before this change.
This commit merely skips the unnecessary calculations.

Signed-off-by: Hakim Filali <[email protected]>
@h-filali h-filali force-pushed the cryptolib-change-test-vec branch from c762629 to 59173d6 Compare October 31, 2025 09:04
@h-filali
Copy link
Contributor Author

h-filali commented Oct 31, 2025 •
edited
Loading

I rebased and updated sw/otbn/crypto/tests/p384_keygen_from_seed_test.s to now check the unmasked secret key result. The masking seems to change slightly as randomness changes with different consumption patterns. To avoid having to update every time we make a change I now check the unmasked result.

Thanks @johannheyszl and @andrea-caforio for reviewing!

@andrea-caforio
Copy link
Contributor

andrea-caforio commented Oct 31, 2025

I rebased and updated sw/otbn/crypto/tests/p384_keygen_from_seed_test.s to now check the unmasked secret key result. The masking seems to change slightly as randomness changes with different consumption patterns. To avoid having to update every time we make a change I now check the unmasked result.

Thanks @johannheyszl and @andrea-caforio for reviewing!

All good.

@h-filali h-filali enabled auto-merge October 31, 2025 10:31
@h-filali h-filali added this pull request to the merge queue Oct 31, 2025
Merged via the queue into lowRISC:master with commit 92f22d3 Oct 31, 2025
47 checks passed
@lowrisc-ci
Copy link

lowrisc-ci bot commented Oct 31, 2025

Successfully created backport PR for earlgrey_1.0.0:

@lowrisc-ci lowrisc-ci bot mentioned this pull request Oct 31, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vogelpi vogelpi vogelpi approved these changes

+2 more reviewers

@johannheyszl johannheyszl johannheyszl approved these changes

@andrea-caforio andrea-caforio andrea-caforio approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CherryPick:earlgrey_1.0.0 This PR should be cherry-picked to earlgrey_1.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[crypto] Use non-zero expected results in some P-256 and P-384 OTBN tests

4 participants

@h-filali @andrea-caforio @vogelpi @johannheyszl